fix(build): derive Astryx surface inventory from @astryxdesign/core - #3883

I reviewed this head and found no blocking issues.

Derives Astryx surface inventory from installed package .d.ts (fail-closed on unparsable, no silent fallback), with correct wrapper/re-implementation classification; hosted test/package/windows_recovery/audit/owner all SUCCESS.

No P0-P3.

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

I reviewed this head and found no blocking issues.

Drift from 5439e35 to 47a2dc2 is pure rebase (includes #3280); inventory derivation logic unchanged. Hosted test: FAILURE on e2e/workhub-layout is unrelated flake (Desktop E2E, not this PR's scope).

No P0-P3.

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

What this PR does

The generated docs/astryx-surface-file-inventory.md could not see most of the design system, for two reasons living in scripts/generate-astryx-surface-inventory.mjs: the known-component set was a hand-maintained 47-name list while @astryxdesign/core ships ~190 components (so Timestamp, ChatMessage, Thumbnail and friends appeared zero times across 213 rows even where they are imported and rendered), and severity only recognized raw button/input/select. This derives the component set by parsing the installed declarations — dist/index.d.ts plus each export * from './Dir' — and adds a reimplementation tier for a local component whose name matches a shipped Astryx twin it does not import.

Verdict

Comment review, not a block: 2 × P1, 2 × P2, 1 × P3, inline.

The diagnosis is right and the derivation half is well built — I checked the parser against the real 0.5.0 declarations and could not find a name it should have caught and missed (194 derived; export type blocks skipped, inline type modifiers handled, export interface XProps not mistaken for a component, SCREAMING constants and useX hooks correctly excluded). Output is byte-stable and locale-independent, and the version is exact-pinned in both manifests and the lockfile.

The two P1s are about the other half. One is mechanical: the gate became install-dependent without moving into the install condition, so an inventory-only PR now red-checks with "run npm install". The other is that the new reimplementation tier, on the current tree, produces two false positives and zero true positives — and one of them is exactly the "wrapper that renders the twin" shape the PR says it excludes.

The thread running through it

The PR names its own root cause well: "a generated artifact whose input is hand-maintained drifts in whichever half the author forgets." It then derives one of the two hand-maintained inputs and leaves the other. MAKA_UI_ASTRYX_REEXPORTS (:146) still hand-lists 9 names while packages/ui/src/index.ts re-exports 34, and the same loop at :281-291 now reads the derived set for one decision and the hand set for the other. So the drift is smaller but structurally identical, and 26 files still under-report.

The second thread is that "wrapper that renders the twin" was implemented as a same-file static-import check, which does not match how this repo actually writes wrappers — markdown.tsx reaches its twin through React.lazy across a file boundary. Widening the exclusion to follow a local re-export/lazy hop, or narrowing the tier to names that are unambiguous rather than generic, would both work; shipping it as-is puts wrong advice into an artifact CI enforces.

Mechanics

The conflict with main is only on the generated file (main deleted runtime-host-project-directory-editor.tsx; 214 rows there against 213 here). Rebase plus npm run astryx:surface-inventory:write resolves it — no hand-editing.

This does not make #2645 materially worse: it conflicts on the same file, its resolution is the same single regenerate command before and after, and it touches package-lock.json, so code=true and the P1 install bug does not hit it. It does mean #2645 can no longer resolve by hand-editing rows.

Note (ungraded)

The churn is understated by omission — the body lists the new names and tiers but not that 66 of 213 rows changed content. The stated totals (blocker 0, reimplementation 2, polish 1, aligned 210) reproduce exactly.

AI use: Claude Code assisted with running the generator, comparing the regenerated artifact, and reading the installed @astryxdesign/core declarations; both P1s were re-verified against the exact head by me, and the analysis, grading, and conclusions are my own. The worktree was restored after regeneration.

Thanks for replacing the hand-maintained Astryx surface map with data derived from the installed package. That is the right ownership boundary and materially reduces drift.

I reviewed exact head 96523606f62a36fe74be19186f369bf7daec3e87. I found one reachable P2 in a reasonable dependency-evolution path: the generator follows only one export * barrel layer. The “nested” fixture currently exercises a direct named export rather than a real Group -> Sub -> Component re-export chain. If Astryx later introduces a second barrel level, valid components can be silently omitted while the broad key/count thresholds still pass.

Could you either traverse the barrel graph recursively with cycle protection and fail-closed handling, or explicitly enforce and test that the installed Astryx package has a one-layer-only export contract? A regression fixture with a genuine multi-level barrel would make the intended boundary executable.

Separately, the current exact head has a failing test check and is conflicting with main, so it will need a refreshed head and green exact-head CI before merge. Those are delivery gates rather than additional code findings.

Review analysis was assisted by Codex and independent @reviewer agents; Astro-Han verified the exact-head implementation, package boundary, reachability, and CI state and owns this review.